Skip to content

Conversation

@LecrisUT
Copy link
Contributor

@LecrisUT LecrisUT commented Apr 24, 2025

TODO:

  • Write new tests or update the old ones to cover new functionality.
  • Update doc-strings where appropriate.
  • Update or write new documentation in packit/packit.dev.

Fixes

Related to

Merge before/after

RELEASE NOTES BEGIN

New abstract types for holding a GitCommit and its changes.

RELEASE NOTES END

@softwarefactory-project-zuul
Copy link
Contributor

@LecrisUT
Copy link
Contributor Author

Ok, basic functionality is in. Should we refine the API here, or do that in a follow-up addressing other suggestions like #924. There is definitely some duplication of the API now, but I will leave it at that for now.

@softwarefactory-project-zuul
Copy link
Contributor

@LecrisUT LecrisUT force-pushed the feat/get_changes branch from 9a67970 to f87861b Compare May 23, 2025 08:36
@softwarefactory-project-zuul
Copy link
Contributor

Copy link
Member

@lbarcziova lbarcziova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks a lot for the contribution, looks really good!

Since the implementation is only for GitHub now, can you please update COMPATIBILITY.md accordingly?

@LecrisUT LecrisUT force-pushed the feat/get_changes branch from f87861b to fccfd71 Compare May 26, 2025 08:17
@LecrisUT
Copy link
Contributor Author

Since the implementation is only for GitHub now, can you please update COMPATIBILITY.md accordingly?

Yeah, would want to nail down more what we want to do with this interface before implementing in the other services. E.g. pull_request.changes.patch duplicates pul_request_.patch.

@softwarefactory-project-zuul
Copy link
Contributor

@lbarcziova
Copy link
Member

@LecrisUT I see, we could have the functionality only in the new class while still preserving the old interface (e.g. pull_request.patch would call the new method) to maintain backward compatibility. But it would require some additional followup work. @mfocko do you want to have a look?

@softwarefactory-project-zuul
Copy link
Contributor

@softwarefactory-project-zuul
Copy link
Contributor

@softwarefactory-project-zuul
Copy link
Contributor

@softwarefactory-project-zuul
Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: new

Development

Successfully merging this pull request may close these issues.

4 participants